Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: RM: Improve init RM message and fix final memory value. #9470

Merged
merged 8 commits into from
Dec 7, 2022
Merged

Fix: RM: Improve init RM message and fix final memory value. #9470

merged 8 commits into from
Dec 7, 2022

Conversation

ajnavarro
Copy link
Member

@ajnavarro ajnavarro commented Dec 7, 2022 •
edited
Loading

Improved init message explaining what RM is doing. Output example:

Initializing daemon...
Kubo version: 0.18.0-dev-a3913d9f9-dirty
Repo version: 12
System version: amd64/linux
Golang version: go1.19.3

Computing default go-libp2p Resource Manager limits based on: 
    - 'Swarm.ResourceMgr.MaxMemory': "8GB"
    - 'Swarm.ResourceMgr.MaxFileDescriptors': 524288

Applying any user-supplied overrides on top.
Run 'ipfs swarm limit all' to see the resulting limits.

Swarm listening on /ip4/127.0.0.1/tcp/4001
Swarm listening on /ip4/127.0.0.1/udp/4001/quic
Swarm listening on /ip4/16.0.0.1/tcp/4001
Swarm listening on /ip4/16.0.0.1/udp/4001/quic
Swarm listening on /ip4/16.1.0.1/tcp/4001
Swarm listening on /ip4/16.1.0.1/udp/4001/quic
Swarm listening on /ip4/16.10.0.1/tcp/4001
Swarm listening on /ip4/16.10.0.1/udp/4001/quic
Swarm listening on /ip4/16.11.0.1/tcp/4001
Swarm listening on /ip4/16.11.0.1/udp/4001/quic
Swarm listening on /ip4/16.12.0.1/tcp/4001
Swarm listening on /ip4/16.12.0.1/udp/4001/quic

Also fixed the computed memory value. When calling Scale(int64(maxMemory), int(numFD)) method we were adding that memory to the previously configured one at System level. Setting System memory and FD to libp2p default ones. Output example using 8GB as set memory now:

ipfs swarm limit system
{
  "Conns": 4611686018427388000,
  "ConnsInbound": 540,
  "ConnsOutbound": 4611686018427388000,
  "FD": 524288,
  "Memory": 8133804032,
  "Streams": 4611686018427388000,
  "StreamsInbound": 8653,
  "StreamsOutbound": 4611686018427388000
}

Signed-off-by: Antonio Navarro Perez antnavper@gmail.com

@ajnavarro ajnavarro mentioned this pull request Dec 7, 2022
Closed
9 tasks
@ajnavarro ajnavarro self-assigned this Dec 7, 2022
BigLep
BigLep reviewed Dec 7, 2022
View reviewed changes
Copy link
Contributor

@BigLep BigLep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ajnavarro. I'm wondering if we should have a message where the resource manager is enabled or not. Something like:

"go-libp2p resource protection disabled"

It emphasizes the purpose (node protection) and makes it clear when someone is forgoing that protectin.

core/node/libp2p/rcmgr_defaults.go Outdated Show resolved Hide resolved
@ajnavarro ajnavarro force-pushed the ajnavarro/fix/better-RM-startup-message-and-fix-memory-values branch from 682be6b to 0c6f669 Compare December 7, 2022 14:31
@ajnavarro
Copy link
Member Author

@BigLep added a message when starting the daemon and RM is disabled.

ajnavarro and others added 6 commits December 7, 2022 17:37
@ajnavarro
Fix: RM: Improve init RM message and fix final memory value.
5d268ae 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Try to fix sharness test.
714091b
@ajnavarro @galargh
Update test/sharness/t0060-daemon.sh
0b96255 
Co-authored-by: Piotr Galar <piotr.galar@gmail.com>
@ajnavarro
Fix sharness checking daemon output
987a9e0 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Requested changes.
9b5822a 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro
Added a message when RM is disabled.
d4ec8e6 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@BigLep BigLep mentioned this pull request Dec 7, 2022
Closed
3 tasks
@ajnavarro ajnavarro force-pushed the ajnavarro/fix/better-RM-startup-message-and-fix-memory-values branch from 0c6f669 to cef695f Compare December 7, 2022 16:39
@ajnavarro
Fix sharness test
6b0b7a0 
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@ajnavarro ajnavarro force-pushed the ajnavarro/fix/better-RM-startup-message-and-fix-memory-values branch from cef695f to 6b0b7a0 Compare December 7, 2022 16:42
Jorropo
Jorropo reviewed Dec 7, 2022
View reviewed changes
test/sharness/t0060-daemon.sh Outdated Show resolved Hide resolved
core/node/libp2p/rcmgr_defaults.go Show resolved Hide resolved
Jorropo
Jorropo approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@ajnavarro ajnavarro merged commit a10d012 into master Dec 7, 2022
@ajnavarro ajnavarro deleted the ajnavarro/fix/better-RM-startup-message-and-fix-memory-values branch December 7, 2022 17:30
@lidel
Copy link
Member

lidel commented Dec 7, 2022

(unrelated) @ajnavarro if you have multiple "wip" commits like https://github.com/ipfs/kubo/pull/9470/commits it is perfectly fine to just squash-merge :)

(makes eyeballing https://github.com/ipfs/kubo/commits/master and inspecting release diff a bit easier for person making the release dance)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galargh galargh galargh left review comments

@Jorropo Jorropo Jorropo approved these changes

@BigLep BigLep BigLep approved these changes

@lidel lidel Awaiting requested review from lidel

@guseggert guseggert Awaiting requested review from guseggert

Assignees

@ajnavarro ajnavarro

Labels
None yet
Projects
No open projects
IPFS Shipyard Team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ajnavarro @lidel @BigLep @galargh @Jorropo